Conversation
bneradt
changed the title
bneradt
changed the title
1b8339e to
678cb46
Compare
bryancall
left a comment
bryancall
left a comment
There was a problem hiding this comment.
Leaving this as a comment rather than a formal review since it is still a draft. There is a lot of good work here, and two things that I think need to come out of this PR before it goes further.
First the good, because the framework itself is well built:
- Replay manifests are collected as first-class pytest items, so each scenario gets its own result and sandbox instead of hiding behind a thin registration wrapper.
- Validation parity with AuTest is preserved rather than quietly dropped.
diags.logis still scanned forERROR:,FATAL:and unrecognized configuration values, background processes are checked for premature exit, and_validate_goldreproduces the{}and backtick wildcard semantics of the old matcher. - Cross-worker port allocation uses a flock'd counter plus a bind probe, which is a genuine improvement over a per-process allocator.
- Sandbox paths are deliberately short and hashed with the rationale tied to
sockaddr_un.sun_path's 108-character limit written down, andprepare_sandboxrefuses tormtreeanything that is not a direct child of the sandbox root. - The consolidation is real rather than lossy, and the framework ships its own unit tests instead of being validated only by the suite it runs.
Two production changes are buried in the test migration
This is my main concern and it is a process point rather than a code-quality one. Copilot declined to analyze this PR because it exceeds its file limit, so these two changes have had no automated review either, and at 2414 files no human is going to find them by reading.
src/api/InkAPI.cc:8240 TSSslClientCertUpdate()'s lookup key changes from swoc::bwprint("{}:{}", cert_path, key_path) to key.assign(cert_path).
I traced this rather than assuming. The inner map is keyed by certificate path alone: SSLConfig.cc:952 sets ctx_key = client_cert, and the only producer of that key, SSLSNIConfig.cc:185, stores a fully resolved absolute path, which is what the cert_update plugin passes via traffic_ctl plugin msg. So on master the composed cert:key string can never match, the API always falls through and returns TS_ERROR without touching anything. After this change it locates the bucket, calls SSLCreateClientContext and swaps the shared context under ctxMapLock.
That is a dead-to-live transition on a public TSAPI affecting shared outbound TLS state. The fix looks correct to me. The problem is that its only covering test, cert_update.test.py, is deleted and rewritten in the same commit range, so a green run cannot distinguish "the API fix works" from "the rewritten test no longer asserts the same thing". This needs to land in its own PR against an unmodified test.
src/proxy/http/HttpTransact.cc:6399-6410 A new proxy.config.http.cache.max_stale_age_percent record, added end to end: RecordsConfig.cc:663, HttpConfig.h:708, OverridableConfigDefs.h:255, a new TS_CONFIG_HTTP_CACHE_MAX_STALE_AGE_PERCENT inserted before TS_CONFIG_LAST_ENTRY in apidefs.h.in:921, cripts/Configs.hpp:104, and the clamp logic in is_stale_cache_response_returnable(). None of it exists on the base and none of the seven commit messages or the PR body mention it.
In fairness: the default is 0 and the arithmetic collapses to the exact master expression at 0, so no behavior change ships. It is still a new proxy.config.* record and a new TSOverridableConfigKey, which is API surface the project supports forever, and it touches the stale-serving threshold used by negative revalidating and open_write_fail_action, which is load-shedding-critical.
One substantive note on the logic itself if it does get its own PR: get_max_age() returns 0, not -1, when a max-age directive is present but zero. So with the percent knob enabled, max_stale_age becomes 0 and any nonzero age fails the returnability test. An origin sending Cache-Control: max-age=0, which is common in front of revalidating origins, stops being served stale entirely during an outage. The documentation covers the absent case but not the present-and-zero case.
The Jenkins entry point is left in a non-working state
ci/jenkins/bin/autest.sh:123
The entire change to this file is one line, -D ./tests/gold_tests to -D ./tests/uranium_tests. Lines 107-108 still resolve AUTEST=/usr/bin/autest and line 123 still executes it. I counted the new tree: tests/uranium_tests has 0 files matching *.test.py, which is AuTest's only collection pattern, against 233 *.test.yaml and 303 test_*.py, and tests/gold_tests no longer exists. tests/pyproject.toml also drops the autest dependency.
So this script either runs a discovery pass over an inventory it cannot see and reports green having executed nothing, or fails for a reason unrelated to the change under test. ci/coverage and ci/regression were correctly updated to call tests/urtest.sh; this one was missed. A CI job that silently runs zero tests is worse than one that fails.
The deprecation story does not hold
CMakeLists.txt:164-165
The PR body says temporary deprecated aliases retain ci-fedora-autest, autest.sh, the old CMake options and targets. I checked each: option(ENABLE_AUTEST) and option(ENABLE_AUTEST_UDS) are replaced outright, the three gates now test ENABLE_URTEST only, and tests/CMakeLists.txt renames the autest, autest_no_install and autest-uds targets with no ALIAS or forwarding target. CMakePresets.json has only ci-fedora-urtest, and there is no tests/autest.sh. The only deprecation shims are AUTEST_SANDBOX, AUTEST_OPTIONS and PYTEST_OPTIONS, and those live inside if(ENABLE_URTEST), so they never fire for a caller passing the old option.
CMake treats an unconsumed -DENABLE_AUTEST=ON as an unused-variable notice, not an error, so an external pipeline configured that way configures successfully, silently skips add_subdirectory(tests), and produces no test target at all. A job using the old preset fails loudly instead, which is the better outcome. Either add real aliases or state plainly in the body that the cut-over is not backward compatible.
Framework items worth fixing while it is still a draft
tests/tools/uranium/runner.py:225is_official_test_container()requires an exactID == "fedora"andVERSION_ID == "44"match, andchoose_docker_mode()defaults to Docker when that fails. When the CI image is bumped to Fedora 45 in some unrelated PR, urtest inside that container stops recognizing it, defaults to Docker mode, finds no docker client, and every shard dies. The safe default when already inside any container is to run directly.tests/tools/uranium/replay.py:657-668_check_metricssleeps a fixed interval then reads each metric exactly once with no retry._check_filesin the same file already does this correctly with a 100ms deadline poll. 17 manifests usemetric_checks, and sleep-then-assert-once under 8-way xdist is precisely the flake pattern this migration is meant to leave behind.tests/tools/uranium/runtime.py:38class RuntimeError(ValueError)shadows the builtin for the whole module, whileprocess.py:29definesProcessError(RuntimeError)against the real builtin. Two unrelated hierarchies with the same spelling in one package.tests/tools/uranium/replay.py:749and:793read_text()with no existence check, so a missing diags or gold file surfaces as a raw traceback into framework internals rather than the intended message. Every sibling path in the same class already guards withif path.exists().
One claim I checked and am withdrawing before anyone chases it: I initially thought ReplayItem.runtest hardcoding is_exclusive=False broke serial_tests.txt for replay manifests. On re-reading it is latent rather than live, so it is worth tidying but it is not causing anything today.
c654ac4 to
c680be9
Compare
|
Thanks for the detailed review. I addressed the feedback and force-pushed the amended commit.
I also normalized direct replay placement, fixed the stale documentation includes, and reran the relevant validation: the documentation build passes with warnings treated as errors, all 371 replay variants passed or reached expected capability skips, and the 51 framework unit tests pass. |
|
[approve ci autest] |
|
I benchmarked this migration on dedicated hardware rather than reasoning about it, because a harness swap of this size deserves numbers. Summary up front: Uranium is 7.4x faster at 32-way parallelism, loses no code coverage, uses less memory, and has fewer flaky tests. Details and methodology below, including the parts that do not favour the new harness. MethodologyTwo hosts, both 32 hardware threads, 30 GB RAM, Fedora, gcc 16.1.1, Proxy Verifier v3.1.3 on both sides (verified identical, checksum
The proxy binary is the same on both sides. I checked this rather than assuming it, because the whole comparison depends on it. After normalizing embedded build paths, both binaries disassemble to 1,341,244 instructions and the instruction streams are identical once addresses are masked. The residual byte differences are relocation displacements from embedded path and timestamp strings of different lengths, spread across 1,265 unrelated symbols in single-byte runs. So every number below is harness overhead, not a proxy change. PerformanceFull suite, same host, same binary:
The gap widens with worker count, which points at the mechanism:
AuTest's worker durations at The cause is that AuTest's load balancing never engages. Uranium's per-test distribution for reference: p50 1.13s, p95 9.17s, max 90.6s. Code coveragegcov plus gcovr 8.6, identical flags and exclusions on both sides, integration suite only:
No coverage is lost. Uranium is fractionally ahead on all three measures. This is the number that matters most, since a 7.4x speedup naturally raises the question of whether the suite is simply doing less. It is not. The denominators are identical on both sides, which independently corroborates the binary-equivalence check above. MemoryPeak resident memory across the whole process tree:
Uranium uses less memory at every level, with the gap widening to about 31% at 16 workers. Neither harness came close to exhausting 30 GB. Caveat worth stating: these are summed RSS across all processes, which double counts pages shared between the many concurrent Traffic Server instances, so treat them as upper bounds. The idle baseline was 0.97 GB, so roughly 92% of each figure is genuine workload. I am re-measuring with proportional set size to remove the double counting and will follow up if it changes the picture. FlakinessSeven AuTest runs and six Uranium runs, across all parallelism levels:
Two observations that only a repeated run surfaces. A flaky test was distorting the timing. AuTest at AuTest does not run a deterministic set of tests. Executed totals across runs were 586, 586, 586, 586, 586, 583, 581. At Test inventoryI mapped all 564 master Exactly one test has no successor: Everything else is verified consolidation, and the counts hold up: the 18 Other findings
OverallThe performance and determinism case is strong and I would not have predicted the margin. The two things I would want resolved before this leaves draft are the Happy to share the raw logs, per-test timing data, or the gcovr HTML reports for either side. |
7fcbc9d to
f3d4785
Compare
Procedural Uranium tests need reusable process ownership and curl targeting that works for multiple ATS instances and Unix sockets. The test tooling layout and command line also made framework code difficult to distinguish from the ATS test inventory. This patch adds fixture-owned ATS factories and ATS-aware curl requests, including UDS support, and converts representative basic coverage to the procedural API. It moves the harness and its unit tests under tests/tools/uranium, updates imports, and lets pytest own selection, parallelism, and collection options while the wrapper handles Docker execution.
The remaining compatibility tests still depended on AuTest process orchestration, which prevented the Uranium suite from being fully native and made parallel execution unreliable. This removes the compatibility backend and converts each test to direct Proxy Verifier replay metadata or a native pytest scenario. It also hardens process startup, logging, reload timestamps, DNS, and timeout handling so the complete suite runs reliably with eight workers. The test guide and runner documentation now describe the replay-first workflow and pytest-native selection and parallelism.
The initial pytest migration left explicitly disabled scenarios as permanent skips and made native curl commands cumbersome to read and write. Repository guidance also retained obsolete AuTest conventions. This patch restores the opt-in scenarios behind a pytest manual marker and adds --run-manual for deliberate execution. It also changes Curl to parse one shell-style argument string, updates every call site, and documents parameter and timeout expectations. This adds collection and service regression coverage and refreshes the Uranium documentation to match the native pytest workflow.
Procedural Uranium support had accumulated unrelated process, client, and assertion behavior in one large module, making changes difficult to isolate while scenarios depended on its public import path. The converted log filename scenario could also read asynchronously written logs before every destination had flushed. This patch addresses this by moving focused implementations into a services package behind the existing tools.uranium.services facade and updating internal imports, documentation, and facade coverage. It also waits for each expected log record before asserting so parallel runs do not race ATS logging.
Replay processing crosses pytest collection, YAML validation, and runtime execution, but the framework directory did not explain those boundaries. Closely related modules were therefore difficult to distinguish. This patch addresses this by documenting the direct replay and procedural call flows, each module and supporting directory, and guidance for placing new framework code.
Split Uranium manifests force readers to move between orchestration metadata and Proxy Verifier traffic when reviewing a single scenario. This patch embeds each file-backed default replay into its collected test YAML and removes the redundant companion files. It also adds an inventory check to keep default replay traffic self-contained. Variant-specific and process-specific replays remain separate where they describe different traffic.
The migration still mixed replay layouts, retained stale documentation references, and left several framework and CI edge cases unresolved. This patch groups direct manifests under replay directories, hardens checks, corrects container and Jenkins execution, and drops unrelated code changes from the migration.
Several converted tests lost AuTest matching and staging semantics or relied on fixed timing. Rebasing also replayed an older migration change that silently removed a newly merged cache configuration feature. This patch restores the upstream cache feature, stages Cripts sources where ATS searches for them, preserves the HTTP/2 counter's numeric variability, and waits for STEK cluster convergence. It also updates the native-test inventory for the test added on master.
Microserver health checks could claim empty header-only lookup keys, making missing-origin responses depend on replay file load order. Uranium also retained successful sandboxes, causing Jenkins to copy gigabytes of irrelevant output when any test failed. This patch gives health checks distinct custom lookup-header values. It removes successful sandboxes after teardown while retaining failed sandboxes for diagnosis.
The Go client can complete before ATS has flushed all access-log entries. Waiting for two generic HTTP/3 entries therefore made the scenario inspect a partially written log and fail intermittently. This patch waits for the required large-POST entry before it checks both expected records. This preserves the assertions while removing the logging race.
The slow-post abort test can fail during ATS startup when the source checkout is mounted under directories that the dropped service user cannot traverse, even though the certificate files themselves are readable. This patch copies the certificate and key into the ATS runroot SSL directory so their permissions and path accessibility match the rest of the test configuration.
Parallel CI can delay access-log buffer flushing beyond ten seconds, and the rate-limit driver can unlink its FIFO before the background holder opens it. These races fail without exercising invalid ATS behavior. This patch allows a bounded 60-second log wait and retains the FIFO until the holder is released so both tests observe their intended state under a loaded VM.
CMCD prefetch validation could issue its cache-hit request before the asynchronous next-hop fill released its cache write lock. Slow CI hosts then observed a legitimate WL_MISS and failed intermittently. This patch replaces fixed sleeps with synchronization on the relevant next-hop transaction and cache write gauge. The hit assertions remain unchanged while the setup no longer races the asynchronous fill.
Augmented assignment made process-output expectations easy to mistake for ordinary assignment. A typo could discard prior checks and let a test lose coverage without an immediate error. This patch introduces read-only stream expectation objects with explicit methods for regex, gold-file, reset, and return-code behavior. It also preserves captured output through clearly named text properties and adds focused misuse and validation coverage.
New AuTests landed on master while the pytest migration was under review, so rebasing without porting them would silently drop their coverage. Recent per-server metric changes also require derived-stat synchronization to avoid racing assertions. This patch converts replay-compatible coverage into combined manifests and extends the native connection-limit scenario for aggregate, hidden, and remap-overridden metrics. It polls derived metrics after shortening the sync interval, preserving the original behavior without fixed timing races.
A new AuTest landed while the pytest migration was under review, so leaving it in the old suite would silently drop its API coverage. This patch converts the custom-listener test to a native Uranium scenario. It uses a Python socket client and verifies the plugin accept callback through its diagnostics.
Uranium's source launcher assumed Docker and relied on Docker-specific markers. Developers using Podman or Apple container could not start the test image, and markerless Apple containers attempted an unavailable nested Docker launch. This patch selects Apple container on macOS and Podman on Linux, with Docker as a fallback. It introduces runtime-neutral flags while preserving the Docker aliases, marks managed launches explicitly, and recognizes Fedora 44 for manually entered Apple containers.
Preserve the coverage added on master while keeping the rebased branch free of AuTest files. Fold replay-friendly cases into existing YAML and use native scenarios for tests requiring custom clients or process control.
Make bare pytest and editor discovery work from the source tree, keep the Python environment locked, and resolve helper scripts from that environment. Support the macOS loader path and document the native workflow.
Wait for live verifier output before inspecting multiplexer copies, and flush HTTP access logs promptly. This removes races exposed by the full parallel test run without weakening the original assertions.
Connection coalescing could assign an origin socket to a queued transaction that did not own its connection-tracker reservation. The live connection was then absent from per-server limits and metrics. This patch keeps the reservation with the in-progress connection and transfers it to the established session. Failed connection attempts release the reservation from the coalescing entry. Fixes: apache#13605
Master added and updated AuTests while this branch was converting the suite to pytest. Leaving those files behind would silently drop their coverage once Uranium becomes the only runner. This patch ports replay-compatible cases to self-contained YAML and rewrites custom-client cases as native scenarios. It also carries forward the accompanying assertions and hardens asynchronous checks exposed by eight-worker runs. Co-authored-by: Codex gpt-5.6-sol xhigh
Worker-number directories made retained Uranium artifacts difficult to navigate because a test owner was not visible at the sandbox root. Failures from parallel runs therefore required extra lookup work. This patch names each item directory from its pytest identity plus a stable digest and places it directly under the shared root. The names remain short enough for ATS Unix sockets, while shared locks and port allocation preserve xdist safety. Co-authored-by: Codex gpt-5.6-sol xhigh
Shortened sandbox names and generated suffixes made test artifacts hard to identify, while automatic cleanup prevented inspecting passing runs. This patch uses full test names and clears named directories on rerun. It adds optional retention of passing sandboxes and rejects colliding names. Unix socket paths stay short independently of artifact paths. Co-authored-by: Codex gpt-6-astra medium
Changes on master left new tests outside the Uranium conversion, while review exposed checks that could pass on incomplete evidence. Shared sandbox names also needed protection against concurrent owners. This patch preserves upstream cache, shutdown, compression, and metric coverage in Uranium. It validates process lifetimes and settled output, rejects unsupported replay assertions, and isolates sandbox ownership without shortening test names. Regression tests exercise the harness failure paths as well as redirected diagnostic output. Co-authored-by: Codex gpt-6-astra medium
57c0465 to
8988b68
Compare
|
@bryancall Thanks for the detailed assertion-by-assertion review. I rebased onto master The review changes are:
On point 7, I deliberately kept the full readable test names requested for this branch instead of restoring truncation. Both JSON-RPC and HTTP Unix sockets use short paths independently of those names; a regression test binds both actual sockets under long sandbox/process names. Unrepresentable filesystem components now fail explicitly. This does not impose a blanket limit for arbitrary future path-limited artifacts; those would need the same short-path treatment. 9–11. The record-triggered reload uses bounded polling, with a fresh trigger marker so an earlier reload cannot satisfy it.
I left the inherited Validation on the rebased branch:
There are no inline review threads to resolve; points 7 and 12 above are explicit responses rather than changes to the requested naming policy or the original slice contract. |
bryancall
left a comment
bryancall
left a comment
There was a problem hiding this comment.
💬 Comment
Eleven of my fifteen asks are fixed, three are partly fixed, and one is not addressed. I am leaving the existing request for changes in force rather than submitting a second one, since that would not change the state, and the branch needs a rebase anyway (mergeable_state: dirty).
Before the open items, the thing I most want to say: you did not just add assertions, you added tests that the new assertions can fail. Four of them, and they are the reason I trust these fixes rather than merely reading them.
Fixed: ask 1, ATS liveness
services/ats.py:380-385. The gate raises when return_code is not None before teardown, and ManagedProcess.return_code is a live poll() on traffic_server itself rather than a cached field or a wrapper, so it observes a crashed proxy. It reaches every instance through ATSFactory.close()'s ExceptionGroup and the fixture's finally, which is broader than the five sites I named. ats.py:646 also inverts the missing-evidence shape so an absent diags log is an error rather than "nothing to check". The parametrized control over [0, 1, -11, -6] is the right test, and status 0 is the important case: a clean early exit was the one most likely to slip past a returncode check.
Fixed: ask 2, terminality before excludes
jsonrpc/config_reload_helpers.py:81 pins it in the helper ("in_progress" not in output), so every caller gets it rather than the ones that remembered. That is a better fix than the per-call-site pin I asked for. Verified against the product rather than assumed: aggregate_status reports IN_PROGRESS when any subtask is in progress and in the mixed case, so a subtask that has not started cannot make the aggregate look settled.
Fixed: ask 3, and better than asked
I pointed at a YAML file. The fix landed in the framework instead: replay.py:766-770 adds the missing excludes branch, and config.py:121-129 rejects unknown keys at load time. So the next misspelled key fails at collection rather than degenerating into "the file exists". The test_process.py control feeds a real violating file and requires the raise.
Fixed: asks 4, 9, 10, 11, 13, 14
Ask 4: test_abuse_shield.py:116-126 derives the expected rule count from the YAML on disk at start time rather than hardcoding it, covering 11 of 11 instances. The \b after rules is load-bearing, since 1 rules would otherwise prefix-match 10 rules.
Ask 9: the time.sleep(3) is gone, replaced by a bounded poll on config status -c all requiring the token present and in_progress absent, raising with the output on timeout.
Ask 10: reload() now has if token == "rpc-greet": assert not errors ahead of the 6010/6011 check, so the strict rule is restored for the scenario that needs it and the relaxed one stays where it belongs.
Ask 11: test_abuse_shield.py:506 restores the pinned literal with the dots escaped, and the absence of .* between address and actions is what makes an inserted field fail.
Ask 13: the recursive chmod is gone, replaced with verify --with-user <me>, which is the right call rather than a weakening: init creates owner-owned files, so verifying as another user would fail for reasons unrelated to any regression. The deliberately unwritable 0400 file expecting exit 70 is the part I would single out, because without it the tightening would have been self-defeating.
Ask 14: all three sub-complaints closed, and the control parametrizes EADDRINUSE against EADDRNOTAVAIL and ENOBUFS to prove only the first retries.
Partly fixed: ask 5, and the gap is mutation-proven
The destructive half is fixed: replay.py:75-76 and :107 put an embedded replay's sandbox under its owner and pass parent=, so a procedural test using uranium_replay no longer erases its own live tree.
Two things remain. runtime.py:177 still has item_sandbox and procedural_sandbox as byte-identical bodies, and item_sandbox's replay_path parameter is documented in its own docstring and never read, which is why a test helper has to fake it as def item_sandbox(self, *_args). A dead parameter threaded through two call sites is how the next person reintroduces the original bug.
More importantly, test_embedded_replay_preserves_services_and_variants monkeypatches ReplayTest.run away and reimplements the prepare_sandbox(..., parent=...) call in its own fake. Reverting replay.py:107 to prepare_sandbox(self.sandbox) leaves the suite at 100 passed. So the test covers the fixture's wiring, not the line that ships. Mitigating: the real path then raises RuntimeConfigError from runtime.py:206, so it is loud rather than silent data loss.
Partly fixed: ask 6, the guard still dies under -n
plugin.py:126 prints to stderr and that does reach the terminal, which is the half that works. Measured on pytest 9.1.1 with xdist 3.8.0: serially, exit 4 and a clean message. Under -n 2 the message appears, and the run still ends in exit 3 with a 48-line INTERNALERROR traceback on stdout terminating in an unrelated assert not crashitem. Because the message goes to stderr and the dump to stdout, the visible tail is the bogus crashitem, and -n is what tests/README.md tells people to use.
test_collision_diagnostic_reaches_terminal asserts only that the substring appears in stdout + stderr, so it passes while the run still crashes, which locks the half-fix in.
The controller sees every worker's collected ids via pytest_xdist_node_collection_finished(node, ids). Doing the duplicate check there, or gating the raise on not hasattr(config, "workerinput"), gives a clean UsageError in both modes.
Partly fixed: ask 8, resolved by exclusion rather than separation
plugin.py:102-107 takes a non-blocking flock on <sandbox>/.session-lock and a second controller gets a UsageError. That is a defensible resolution and I am not asking you to change the approach. Two holes in it:
The handler catches only BlockingIOError, so on a filesystem where flock is unsupported or unserved (NFS without lockd gives ENOLCK) the OSError escapes pytest_sessionstart as an INTERNALERROR rather than a message. _short_sandbox defaults to /tmp, but --sandbox and ATS_URTEST_SANDBOX are user-supplied and documented.
The collision guard also only sees co-collected items. sandbox_name discards the file path, so a/test_x.py::test_foo and b/test_y.py::test_foo both map to test_foo. A full run catches that; two selective runs each collect one item, never trip the guard, share the deterministic root, and the second rmtrees the first's directory. That matters most under --keep-sandboxes, whose whole purpose is retaining it.
Not addressed: ask 12, the slice alternation
test_slice_stale_generation.py:196-204 is unchanged; the +2 lines are a comment explaining the race. I accept the race is real, and I am not pressing for the single original message. Two things are still worth a line of work.
The alternation is unbracketed, so re.search reads it as (Failed to find a well-formed, completed HTTP response: PARSE_INCOMPLETE) OR (Content-Length body underrun for key mixed), and only the second branch carries key mixed. The first would be satisfied by a PARSE_INCOMPLETE on any key in a multi-key run. Bracketing it costs nothing.
The surrounding assertions also carry more weight than I gave them credit for, so I want to be fair about what is actually exposed: expected_return_code=1 means a complete well-formed response makes the client exit 0 and fail, :203 pins the failure to the mixed key, and assert_mismatch_diagnostics(both_blocks=True) pins two Mismatch/Bad block Content-Range lines with specific blk_range and etag_got. What survives unguarded is narrow: a change in where slice aborts, not whether it aborts.
Non-blocking, from the same delta
process.py:221-231: stop() still calls wait() and discards the reaped status without comparing it to return_codes, and the new gate fires before the SIGTERM. So anything that goes wrong during shutdown is unexamined: a crash in a plugin's shutdown path or a sanitizer report at exit leaves the test green, since close() greps diags_log and not traffic_out. ATS.wait() already validates, which is why the one test that calls it is the one that would catch this. Ask 1 is fixed by wrapping ManagedProcess, not by fixing it, so any future long-running service reintroduces the original gap.
replay.py:801: disable_log_checks is consumed only inside _validate_ats_logs, which only the replay path reaches, so procedural tests have no ERROR: diags check at all, only the FATAL: grep. Five files still pass the dead option. Dropping it from the two ssl reload tests reads as re-enabling log checking, and nothing was re-enabled.
ats.py:380: not self._allow_fatal_diagnostics now gates the unexpected-exit check as well as the fatal-diagnostic one, so a future test that merely tolerates a FATAL line would silently also stop noticing that ATS died. Worth a separate flag.
ats.py:646 uses a bare assert for the missing-log message where the line beside it raises AssertionError explicitly. Under -O the message is lost and the failure becomes a FileNotFoundError traceback.
Method note
Fifteen asks are more than one reader can hold, so this went through the review panel with the asks split by area, and I verified each verdict against the source before repeating it. Two agents disagreed about ask 2 and the disagreement was semantic rather than factual; three asks were not covered by any agent and I checked those myself, which is how 9 and 10 got their verdicts. Not verified by execution on my side: I did not run the suite, so the -n crash and the mutation result are the panel's measurements, reproduced from their commands rather than re-run by me.
This replaces Traffic Server's AuTest end-to-end suite with Uranium, a pytest
test framework for Proxy Verifier replays and native Python scenarios. There is
no AuTest compatibility backend in the resulting test suite.
Summary
<scenario>.test.yaml; eachmanifest contains both its
urtestconfiguration and replay sessionspytest scenario classes with an explicit
run()entry pointand shared/exclusive scenarios
under
tests/tools/uraniumurtest.sh, Uranium CMakeoptions and targets, and
urtestreplay metadataDirect replay manifests live in an existing
replay/orreplays/directorywhen a test tree has one. The
at_headersmanifest intentionally remains nextto its plugin assets.
Developer workflow
Run selected tests with pytest's normal selection and parallelism options:
A configured tree generates
<build>/tests/urtest.shfor running against itsinstalled ATS tree. Manual tests are skipped by default and can be selected
explicitly with
--run-manual -k <expression>.Container behavior
The source-tree runner defaults to
ci.trafficserver.apache.org/ats/fedora:44, performs an incremental dedicatedbuild/install, and runs pytest there. It avoids nested Docker whenever it is
already inside any container.
--run-in-dockerand--no-run-in-dockeroverride that choice.
The official Fedora image also enables the optional
cdifflibacceleration forlarge gold-file comparisons. Other environments use it when installed and
fall back to the standard-library
difflibimplementation otherwise.Compatibility
This is an intentional cutover rather than a deprecation shim. The AuTest
runner, backend, CMake targets, presets, and options are removed. CMake reports
a fatal migration message if removed
ENABLE_AUTEST,ENABLE_AUTEST_UDS,AUTEST_SANDBOX,AUTEST_OPTIONS, orPYTEST_OPTIONSvariables are supplied.Existing CI scripts now invoke Uranium and pass pytest's
-kand-noptions.SHARDandSHARDCNTcontinue to distribute pytest items across CI shards.Validation
failure for the pre-existing
TSSslClientCertUpdatedefectchecks passed